Skip to content

docs: remove todo from code#3202

Open
csviri wants to merge 2 commits intooperator-framework:mainfrom
csviri:remove-todo
Open

docs: remove todo from code#3202
csviri wants to merge 2 commits intooperator-framework:mainfrom
csviri:remove-todo

Conversation

@csviri
Copy link
Collaborator

@csviri csviri commented Mar 5, 2026

I use todos for development, I alway got caught up on this what is usually unrelated. But I beleive generally is a bad practice to have todos in code when not work in progress.

( https://www.javathinking.com/blog/how-to-manage-todo-programming-stuff/?utm_source=chatgpt.com )

maybe we should rather mar it in the related issue, that needs to be updated in JOSDK

Signed-off-by: Attila Mészáros a_meszaros@apple.com

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
@csviri csviri requested review from Copilot and metacosm and removed request for Copilot March 5, 2026 11:18
@openshift-ci openshift-ci bot requested a review from xstefank March 5, 2026 11:18
@metacosm
Copy link
Collaborator

metacosm commented Mar 5, 2026

If you remove todos from code then create associated issues to fix the related problem. Removing the todo doesn't help anything related to technical debt, it just hides it so I don't see how this is an improvement.

@csviri
Copy link
Collaborator Author

csviri commented Mar 5, 2026

If you remove todos from code then create associated issues to fix the related problem. Removing the todo doesn't help anything related to technical debt, it just hides it so I don't see how this is an improvement.

There is already an associated issue in fabroc8 client linked in the code, you mean should we create also in JOSDK repo?

see: fabric8io/kubernetes-client#6314

I don't see how this is an improvement

Again, see here explained how TODOs pollutes the code: https://www.javathinking.com/blog/how-to-manage-todo-programming-stuff/?utm_source=chatgpt.com

@csviri csviri self-assigned this Mar 5, 2026
@metacosm
Copy link
Collaborator

metacosm commented Mar 5, 2026

If you remove todos from code then create associated issues to fix the related problem. Removing the todo doesn't help anything related to technical debt, it just hides it so I don't see how this is an improvement.

There is already an associated issue in fabroc8 client linked in the code, you mean should we create also in JOSDK repo?

Yes, having an issue in the Fabric8 client doesn't help the quality of the code here, as the todo is to replace the local implementation by the one provided by Fabric8 once it's available. That should be quite obvious, reading the todo message.

Again, see here explained how TODOs pollutes the code: https://www.javathinking.com/blog/how-to-manage-todo-programming-stuff/?utm_source=chatgpt.com

And reading that blog, it's quite obvious that this todo fits their best practices.

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Copilot AI review requested due to automatic review settings March 5, 2026 21:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes in-code TODO notes from GroupVersionKindPlural#getDefaultPluralFor, aligning the codebase with the stated preference to track follow-ups in issues rather than leaving TODOs in non-WIP code.

Changes:

  • Deleted TODO and external link comments above getDefaultPluralFor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@csviri
Copy link
Collaborator Author

csviri commented Mar 5, 2026

Yes, having an issue in the Fabric8 client doesn't help the quality of the code here, as the todo is to replace the local implementation by the one provided by Fabric8 once it's available. That should be quite obvious, reading the todo message.

But that is the point, there is a message that you read all the time. Even pops up in a todo list. As formulated nicely in that blogpost this is waste of time, where you go back all the time. While the issue in fabric8 is lingering there for more than 1.5 year, and might be just never merged. Yet this pops up all the time in the todo list. So basically that quite bad.

I changed the PR to remove the whole comment, will rather have a pointer in that issue, to change this in JOSDK if that ever gets merged.

(Not talking about that actually this whole concept of GVKP is very questionable, not used in JOSDK at all)

@csviri
Copy link
Collaborator Author

csviri commented Mar 5, 2026

But I don't want to spend to much energy on this, if we all don't agree on this approach, that is also fine by me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants